Skip to content

Go: fold a package's type into one node when its methods span files (#3399) - #3400

Open
xiongjianxu wants to merge 1 commit into
Graphify-Labs:v8from
xiongjianxu:feat/go-package-type-fold
Open

Go: fold a package's type into one node when its methods span files (#3399)#3400
xiongjianxu wants to merge 1 commit into
Graphify-Labs:v8from
xiongjianxu:feat/go-package-type-fold

Conversation

@xiongjianxu

Copy link
Copy Markdown
Contributor

Fixes #3399.

A Go type whose methods live in other files of the same package became several nodes, one per
file, each owning only the methods declared there — and the declaring file's node owning none of
them. extract_go mints the receiver's type in every file that declares a method on it, keyed
_make_id(pkg_scope, receiver_type), so those nodes share one id and differ only in
source_file; _disambiguate_colliding_node_ids reads that as a collision between distinct
entities and salts each one with its own path.

Change

One pass, _merge_go_package_types, run immediately after _merge_decl_def_classes and for the
same reason: before disambiguation, so it sees one source_file per id and leaves it alone. The
colliding nodes already share an id, so every edge already points at the survivor — only the
redundant duplicates are dropped, no edge moves.

The declaring file wins, identified as the only file with a contains edge to the type. When the
declaration lives outside the corpus the whole group is method-only; one node is still the
answer, so it folds on the lowest path.

Guards against a false merge

  • One directory. The id folds in only the directory's name, so a/svc and b/svc collide
    on id while being different packages. Those stay split, as today.
  • _test.go stops the fold. Its package clause may be the separate external svc_test
    package declaring a type of the same name. extract_go derives pkg_scope from the directory
    and never parses the package clause, so same-directory is not evidence of same package there.
  • Bare type labels only. A Go type name holds neither . nor (, while every other label
    the extractor mints does (Run(), .Close(), the file node's a.go), so a package whose
    helper function and a method share a name is untouched.

Verification

Before, on the three-file package from the issue: three Server nodes, .Close() on the b.go
one and .Save() on the c.go one. After: one svc_server sourced at a.go owning both, with
the parameter_type reference from Run() pointing at it.

tests/test_go_package_type_fold.py — 6 tests: methods across files fold onto one owner; a
reference reaches the node that owns the methods; the declaring file survives a lower-sorting
method file; and the three guards above. The three positive tests fail on v8 and pass here;
the guard tests pass both ways, pinning the no-regression half.

Full suite: 5315 passed, 93 skipped, ruff check graphify tests clean. The only failures in this
environment are the pre-existing tests/test_ollama_retry_cap.py ones (no openai module
installed), which fail identically on plain v8.

Relation to #3395

Independent of it and based on v8. The Go member-call resolver added there needs exactly one
declaration of the receiver's type, so on a multi-file package it currently bails; this fold is
what lets it fire. Neither PR needs the other to land first.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. No changes could be formally verified in this run.


Graphify review — findings

Adds _merge_go_package_types, run right after _merge_decl_def_classes and before id disambiguation, to fold the duplicate Go type nodes that extract_go mints once per file declaring a method on a package type into a single node, keeping the declaring file (type X) as the survivor and preserving each dropped method's own source_file. It only collapses bare type labels among .go files sharing one directory, and deliberately leaves splits in place for same-named packages in different directories, _test.go external test packages, and function/method name collisions, since the id folds in the directory name and the package clause isn't parsed. Without it, disambiguation fragments one type across partial nodes and downstream single-definition guards bail on the resulting ambiguity.

No blocking issues surfaced. 7 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 2043 functions depend on the 371 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 546 callers, 44 callees
  • new: _rebuild_code() — 115 callers, 51 callees
  • new: _extract_generic() — 18 callers, 26 callees
  • new: extract_js() — 85 callers, 4 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: dispatch_command() — 2 callers, 124 callees
  • new: extract_objc() — 27 callers, 9 callees
  • new: _resolve_js_module_path() — 27 callers, 6 callees
  • …and 44 more — each is listed as a finding

Verification — 2043 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 1878 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify extract.

The verifier did not have enough to check extract, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `cache_root` is annotated `Path | None` — outside the synthesizable primitive/collection set

· 52 more finding(s) on lines outside this diff (see the check run).

…les (Graphify-Labs#3399)

extract_go keys a type on its package directory, so every file declaring a
method on it mints the type again under the same id. Disambiguation then splits
those apart by path, and the type ends up fragmented into several partial nodes,
each owning some of its methods: the declaring file's node owns none of them,
and every single-definition guard downstream reads the set as an ambiguity.

Fold the id-collision before disambiguation, the same point and the same shape
as _merge_decl_def_classes for C/C++/ObjC. Guards: all members must be .go
files in one directory (the id folds in only the directory's name), and a
_test.go member stops the fold, since its package clause may be a separate
external test package. The declaring file wins; no edge re-pointing is needed
because the group already shares one id.
@xiongjianxu
xiongjianxu force-pushed the feat/go-package-type-fold branch from fdf5fe9 to 27746d6 Compare September 11, 2026 05:35

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.

Formal verification. 3 change(s) tested, no difference found (not proven).


Graphify review — findings

Adds _merge_go_package_types, run just before ID disambiguation, to fold the duplicate nodes extract_go mints for a Go type whose methods are spread across files (since it keys the type on its package directory) into a single node — the declaring file's node wins, dropped copies keep their own source_file, and no edges need re-pointing since the group already shares one id. Guards keep genuinely distinct nodes split: only .go files sharing one directory fold, _test.go members abort the fold (their package clause may be a separate svc_test package), and only bare type labels are considered so same-named functions and methods survive. Includes tests covering the merge, reference reachability, declaring-file precedence, and the split cases for directory-name collisions across packages and external test packages.

Worth a look

  • Go type fold drops distinct build-tagged declarationsgraphify/extractors/resolution.py:2494 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 2043 functions depend on the 371 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 546 callers, 44 callees
  • new: _rebuild_code() — 115 callers, 51 callees
  • new: _extract_generic() — 18 callers, 26 callees
  • new: extract_js() — 85 callers, 4 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: dispatch_command() — 2 callers, 124 callees
  • new: extract_objc() — 27 callers, 9 callees
  • new: _resolve_js_module_path() — 27 callers, 6 callees
  • …and 44 more — each is listed as a finding

Verification — 2043 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 1878 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

101 of 256 test file(s) selected (39%) via static blast radius.

  • tests/test_astro_extraction.py — impact
  • tests/test_astro_import_ids.py — impact
  • tests/test_build.py — impact
  • tests/test_builtin_global_type_refs.py — impact
  • tests/test_case_sensitive_resolution.py — impact
  • tests/test_cjs_module_extension.py — impact
  • tests/test_cpp_nested_and_cli.py — impact
  • tests/test_cpp_objc_cross_file_calls.py — impact
  • tests/test_cross_extension_reexport_self_cycle.py — impact
  • tests/test_cross_language_call_resolution.py — impact
  • tests/test_cross_repo_member_calls.py — impact
  • tests/test_csharp_call_site_generic_args.py — impact
  • tests/test_csharp_enum_members.py — impact
  • tests/test_csharp_field_generic_args.py — impact
  • tests/test_csharp_interface_dispatch.py — impact
  • tests/test_csharp_member_calls.py — impact
  • tests/test_csharp_member_nodes.py — impact
  • tests/test_csharp_object_creation.py — impact
  • tests/test_csharp_partial_classes.py — impact
  • tests/test_csharp_type_resolution.py — impact
  • tests/test_definition_file_portability.py — impact
  • tests/test_detect.py — impact
  • tests/test_dotnet.py — impact
  • tests/test_duplicate_annotation_edges.py — impact
  • tests/test_extract.py — impact
  • tests/test_extract_cache_location.py — impact
  • tests/test_file_label_disambiguation.py — impact
  • tests/test_file_node_id_spec.py — impact
  • tests/test_forwarding_review_findings.py — impact
  • tests/test_go_builtin_call_targets.py — impact
  • tests/test_go_package_type_fold.py — impact, changed-test
  • tests/test_go_qualified_resolution.py — impact
  • tests/test_import_extension_resolution.py — impact
  • tests/test_import_self_loops.py — impact
  • tests/test_imported_export_forwarding.py — impact
  • tests/test_incremental.py — impact
  • tests/test_indirect_call_arrow_single_param_shadow.py — impact
  • tests/test_indirect_call_catch_binding_shadow.py — impact
  • tests/test_indirect_call_external_import_shadow.py — impact
  • tests/test_indirect_call_for_of_binding_shadow.py — impact
  • tests/test_indirect_call_function_expression_shadow.py — impact
  • tests/test_indirect_call_nested_closure_shadow.py — impact
  • tests/test_indirect_dispatch.py — impact
  • tests/test_indirect_dispatch_assign_return.py — impact
  • tests/test_indirect_dispatch_getattr.py — impact
  • tests/test_inferred_confidence_rubric.py — impact
  • tests/test_inherited_field_receivers.py — impact
  • tests/test_java_member_calls.py — impact
  • tests/test_java_type_resolution.py — impact
  • tests/test_js_callback_calls.py — impact
  • … and 51 more

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

Formal verification

Could not verify: Could not verify build.

The verifier did not have enough to check build, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify build\_merge.

The verifier did not have enough to check build\_merge, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `graph_path` is annotated `str | Path | None` — outside the synthesizable primitive/collection set

Could not verify: Could not verify merge\_raw\_extraction.

The verifier did not have enough to check merge\_raw\_extraction, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `graph_path` is annotated `str | Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify dispatch\_command.

The verifier did not have enough to check dispatch\_command, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)

Could not verify: Could not verify deduplicate\_entities.

The verifier did not have enough to check deduplicate\_entities, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify \_pick\_winner.

The verifier did not have enough to check \_pick\_winner, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify \_is\_noise\_dir.

The verifier did not have enough to check \_is\_noise\_dir, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `parent` is annotated `'Path | None'` — outside the synthesizable primitive/collection set

Could not verify: Could not verify extract.

The verifier did not have enough to check extract, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `cache_root` is annotated `Path | None` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_import\_php.

The verifier did not have enough to check \_import\_php, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)

No difference found (not proven): No behavior difference found in \_normalize\_ts\_import\_types (not a proof).

The verifier ran both versions of \_normalize\_ts\_import\_types on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.

Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.

Note: An input the sampler did not try could still differ.

Could not verify: Could not verify \_resolve\_rescued\_specifier.

The verifier did not have enough to check \_resolve\_rescued\_specifier, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify extract\_bash.

The verifier did not have enough to check extract\_bash, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify extract\_dart.

The verifier did not have enough to check extract\_dart, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_extract\_generic.

The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_js\_extra\_walk.

The verifier did not have enough to check \_js\_extra\_walk, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify \_apply\_symbol\_resolution\_facts.

The verifier did not have enough to check \_apply\_symbol\_resolution\_facts, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_collect\_python\_symbol\_resolution\_facts.

The verifier did not have enough to check \_collect\_python\_symbol\_resolution\_facts, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_resolve\_js\_module\_path.

The verifier did not have enough to check \_resolve\_js\_module\_path, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `start_dir` is annotated `Path | None` — outside the synthesizable primitive/collection set

Could not verify: Could not verify extract\_rust.

The verifier did not have enough to check extract\_rust, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify extract\_sql.

The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set

No difference found (not proven): No behavior difference found in \_pinned\_python (not a proof).

The verifier ran both versions of \_pinned\_python on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.

Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.

Note: An input the sampler did not try could still differ.

Could not verify: Could not verify install.

The verifier did not have enough to check install, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)

Could not verify: Could not verify generate\_community\_labels.

The verifier did not have enough to check generate\_community\_labels, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous

Could not verify: Could not verify \_atomic\_replace.

The verifier did not have enough to check \_atomic\_replace, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `path` is annotated `'str | Path'` — outside the synthesizable primitive/collection set

No difference found (not proven): No behavior difference found in \_query\_graph\_text (not a proof).

The verifier ran both versions of \_query\_graph\_text on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.

Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.

Note: An input the sampler did not try could still differ.

· 52 more finding(s) on lines outside this diff (see the check run).

@xiongjianxu

Copy link
Copy Markdown
Contributor Author

The push was an author-identity rewrite (my commits carried an email not linked to my GitHub account); the tree is byte-identical to the previously reviewed head.

On the finding that the package-level fold drops distinct build-tagged declarations: it does not. Two files in one package, each with a //go:build tag and each declaring type Client struct plus one method, extract to a single folded Client node that keeps both:

nodes: Client @client_linux.go
edges: Client --contains--> .Open()   (client_linux.go)
       Client --contains--> .Close()  (client_darwin.go)

Both methods and both contains edges survive; the only thing collapsed is the scalar source_file attribute, which can name one file only. That is also the semantically right answer: build tags are mutually exclusive, so at most one variant is ever compiled for a given target, and a call to c.Open() in that package resolves to the one Client the build selected. Splitting the node per file would instead produce two Client declarations in one package, which is what trips the len(type_defs) != 1 ambiguity guard and would silently lose the edges this PR is adding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Go: a package's type is fragmented into one node per file that declares a method on it

1 participant